Hi @mykaul. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Looks reasonable to me.

can we keep these changes in two separate commits?
Do you have pointers to some docs that can help understand the effect?

@abhinavdahiya - I guess we can have them in two commits, not sure it's worth the effort though.
I have enough experience with libvirt and KVM to know that both are useful and provide better performance - certainly the CPU pass-through - it allows the guest to utilize the full potential of the host CPU capabilities.
From https://wiki.openstack.org/wiki/LibvirtXMLCPUModel :

"host-passthrough" - this causes libvirt to tell KVM to passthrough the host CPU with no modifications. The difference to host-model, instead of just matching feature flags, every last detail of the host CPU is matched. This gives absolutely best performance, and can be important to some apps which check low level CPU details, but it comes at a cost wrt migration. The guest can only be migrated to an exactly matching host CPU.

One useful thing for example is this guarantees that if the host has RDRAND so does the guest.

I've left out the virtio console part (it's less important), and just have the host-passthrough CPU type.

51b497d has the commit title Use host-passthrough CPU type, virtio console i think we can drop text regarding the virtio.

Also you can include

From https://wiki.openstack.org/wiki/LibvirtXMLCPUModel :

"host-passthrough" - this causes libvirt to tell KVM to passthrough the host CPU with no modifications. The difference to host-model, instead of just matching feature flags, every last detail of the host CPU is matched. This gives absolutely best performance, and can be important to some apps which check low level CPU details, but it comes at a cost wrt migration. The guest can only be migrated to an exactly matching host CPU.

in commit message body (https://github.com/openshift/installer/blob/master/CONTRIBUTING.md#commit-message-format)

Thanks! 😇

Updated.

/ok-to-test

Why is this just for the bootstrap node, and not for the master domains too? Also, if this is something folks want to set on libvirt, does it make sense to add it to the cluster-API provider too, either as a configurable or hard-coded setting?

@wking - it should be for the master node as well! Will add.
I'm not sure what the cluster-api-provider-libvirt project is... I can look at it, but it's unrelated to this PR, I assume?

I'm not sure what the cluster-api-provider-libvirt project is... I can look at it, but it's unrelated to this PR, I assume?

The libvirt provider is in charge of creating nodes to fulfil machine and machine-set configuration. At the moment, that means it creates the worker nodes to fulfil the worker machine-set. Eventually it may also be in charge of creating master nodes (if we work out a way to handle etcd bootstrapping and new-master joining). #792 is also related to this, using the cluster-API machine configurations to figure out the Terraform configuration.

/lgtm

This looks fine for now. We can sort out the actuator stuff later.

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, mykaul

The full list of commands accepted by this bot can be found here.

The pull request process is described here